Make Vertex AI evaluation optional to avoid litellm bloat - #69749
Make Vertex AI evaluation optional to avoid litellm bloat#69749Vamsi-klu wants to merge 7 commits into
Conversation
|
Reviewers: @shahar1 (google provider CODEOWNER). Breaking change: evaluation extra now optional to avoid litellm bloat. Changelog entry added, provider docs regenerated via update-providers-dependencies. Fixes #69323 Drafted-by: Muse Spark 1.1; reviewed by @Vamsi-klu before posting |
8d5aab9 to
b54b2d6
Compare
|
Quickest fix: git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-leaseAutomated nudge — ignore if you're not ready to rebase. This comment is updated in place on future |
|
@Vamsi-klu This PR has been converted to draft because it does not yet meet our Pull Request quality criteria. Issues found:
What to do next:
Converting a PR to draft is not a rejection — it is an invitation to bring the PR up to the project's standards so that maintainer review time is spent productively. There is no rush — take your time and work at your own pace. We appreciate your contribution and are happy to wait for updates. If you have questions, feel free to ask on the Airflow Slack. Note: This comment was drafted by an AI-assisted triage tool and may contain mistakes. Once you have addressed the points above, an Apache Airflow maintainer — a real person — will take the next look at your PR. We use this two-stage triage process so that our maintainers' limited time is spent where it matters most: the conversation with you. |
1506a46 to
ce7370f
Compare
5820c69 to
9ee89ab
Compare
Base dependency on google-cloud-aiplatform[evaluation] forced litellm, scikit-learn, tokenizers for all users. Move evaluation extra behind optional provider extra 'evaluation'. Add lazy import guard with AirflowOptionalProviderFeatureException in GenerativeModelHook. Users needing RunEvaluationOperator should install apache-airflow-providers-google[evaluation]. Fixes: apache#69323
Use TYPE_CHECKING guard and Any fallback to satisfy mypy when evaluation extra not installed. Runtime guard still raises AirflowOptionalProviderFeatureException before using None types. Fixes mypy failure in CI for apache#69323
The provider now keeps evaluation dependencies out of the base install, so the missing-extra path needs regression coverage and operator docs that show users how to opt back in.
Reloading the hook module rebound GenerativeModelHook, so tests that imported the class at collection time patched a different class object and hit the real EvalTask. Patching the import-guard variable tests the same behavior without touching module state.
9ee89ab to
1141e4f
Compare
|
|
||
| # For no Pydantic environment, we need to skip the tests | ||
| pytest.importorskip("google.cloud.aiplatform_v1") | ||
| pytest.importorskip("vertexai.preview.evaluation") |
There was a problem hiding this comment.
I noticed that the CI image installs via uv sync --all-packages --group ci-image (Dockerfile.ci, no --all-extras), so this skip fires there, and the only tests in these two files are test_run_evaluation and TestVertexAIRunEvaluationOperator::test_execute, i.e. exactly the paths this PR guards. Is that intended?
There was a problem hiding this comment.
Good catch, thanks for pointing this out! You are right, this was not intended.
The pytest.importorskip("vertexai.preview.evaluation") in those two files would fire in the default CI image since it uses uv sync --all-packages --group ci-image without extras, and those files only contain test_run_evaluation and test_execute. So we would end up with no coverage for the evaluation path in CI.
I pushed a fix in 4c25fa3:
- Removed the
importorskipforvertexai.preview.evaluationfrom bothtest_generative_model.pyfiles - Wrapped the
MetricPromptTemplateExamplesimport in a try/except and fall back to aMagicMockwith thePointwisevalues when the evaluation extra is not installed. The tests already mockget_eval_taskandget_generative_model, so this lets the wiring be tested without needing the reallitellmstack - Also cleaned up the leftover merge markers in
changelog.rst
Verified locally that pytest --collect-only now shows 2 tests collected and both pass, and the test_generative_model_optional_evaluation.py tests for the missing extra still pass. Let me know if you would prefer to keep the skip and instead run these tests in a job with the evaluation extra, happy to adjust.
Address review comment that pytest.importorskip('vertexai.preview.evaluation')
caused the only tests in two files to be skipped in the default CI image
which installs via uv sync --all-packages --group ci-image without extras.
Replace the skip with a try/except import that falls back to a MagicMock
providing MetricPromptTemplateExamples.Pointwise attributes. Tests now
execute with mocks in default CI, while the missing-extra path remains
covered by test_generative_model_optional_evaluation.
Also fix changelog merge markers left from previous rebase.
Problem
apache-airflow-providers-googlepulledgoogle-cloud-aiplatform[evaluation]into the base provider install. That extra brings in evaluation-only dependencies such aslitellm,scikit-learn,huggingface-hub, andtokenizersfor users who may only need unrelated Google services.The only code path that needs
vertexai.preview.evaluationis Vertex AI model evaluation throughGenerativeModelHook.get_eval_task(),GenerativeModelHook.run_evaluation(), andRunEvaluationOperator.What Changed
google-cloud-aiplatform>=1.155.0.evaluationprovider extra that installsgoogle-cloud-aiplatform[evaluation]>=1.155.0.AirflowOptionalProviderFeatureExceptionwith an install instruction when evaluation APIs are used without the extra.uv.lock.evaluationextra in the Vertex AI operator docs.Why The Follow-Up Commit Exists
The feedback was correct: the original PR proved the opt-in path but did not directly test the failure mode when the extra is missing, did not document the extra next to
RunEvaluationOperator, and left the lockfile inconsistent with the dependency split.Impact
RunEvaluationOperatoror direct Vertex AI evaluation hook methods must installapache-airflow-providers-google[evaluation].Fixes: #69323
Testing
UV_CACHE_DIR=/tmp/uv-cache-69749 uv run --python 3.11 ruff format providers/google/tests/unit/google/cloud/hooks/vertex_ai/test_generative_model_optional_evaluation.pyUV_CACHE_DIR=/tmp/uv-cache-69749 uv run --python 3.11 ruff check --fix providers/google/tests/unit/google/cloud/hooks/vertex_ai/test_generative_model_optional_evaluation.pyAIRFLOW_HOME=/tmp/airflow-home-69749 UV_CACHE_DIR=/tmp/uv-cache-69749 uv run --python 3.11 --project providers/google pytest providers/google/tests/unit/google/cloud/hooks/vertex_ai/test_generative_model_optional_evaluation.py --with-db-init -xvs2 passed, 1 warningAIRFLOW_HOME=/tmp/airflow-home-69749 UV_CACHE_DIR=/tmp/uv-cache-69749 uv run --python 3.11 --project providers/google --extra evaluation pytest providers/google/tests/unit/google/cloud/hooks/vertex_ai/test_generative_model.py providers/google/tests/unit/google/cloud/operators/vertex_ai/test_generative_model.py -xvs2 passed, 1 warningWas generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-5) following the guidelines
Drafted-by: Codex (GPT-5); reviewed by @Vamsi-klu before posting